Skip to content

feature(splitter): Added support for collapsed panes state - #2299

Open
rkaraivanov wants to merge 9 commits into
masterfrom
rkaraivanov/splitter-collapsed-panes-state
Open

feature(splitter): Added support for collapsed panes state#2299
rkaraivanov wants to merge 9 commits into
masterfrom
rkaraivanov/splitter-collapsed-panes-state

Conversation

@rkaraivanov

Copy link
Copy Markdown
Member

Description

Added support for collapsed panes state in the splitter component. This enhancement allows developers to programmatically control the collapsed state of panes, enabling more dynamic and interactive layouts.
The feature includes methods to collapse and expand panes, as well as events to listen for state changes.

Type of Change

  • New feature (non-breaking change that adds functionality)

Related Issues

Closes #2297

Checklist

  • My code follows the project's coding standards
  • I have tested my changes locally
  • I have updated documentation if needed

Added support for collapsed panes state in the splitter component.
This enhancement allows developers to programmatically control the collapsed state of panes,
enabling more dynamic and interactive layouts.
The feature includes methods to collapse and expand panes, as well as events to listen for state changes.

Closes #2297

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds public API support for controlling and observing collapsed/expanded pane state in igc-splitter, aligning the component with the requested “persist/restore layout” scenarios.

Changes:

  • Introduces startCollapsed / endCollapsed properties (reflected as attributes) to programmatically control pane collapsed state.
  • Adds igcExpansionChanged event and corresponding event args types/exports.
  • Updates Storybook stories, tests, and changelog to document and validate the new behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
stories/splitter.stories.ts Adds Storybook controls + a persisted-layout example using the new properties/event.
src/index.ts Re-exports the new IgcSplitterExpansionChangedEventArgs type from the public entry.
src/components/splitter/types.ts Adds the igcExpansionChanged event typing to the splitter event map.
src/components/splitter/splitter.ts Implements collapsed-state properties and emits igcExpansionChanged for user interactions.
src/components/splitter/splitter.spec.ts Adds unit tests for collapsed-state properties, attribute reflection, and the new event behavior.
CHANGELOG.md Documents the new splitter collapsed-state API and event in the changelog.
Comments suppressed due to low confidence (2)

src/components/splitter/splitter.ts:513

  • startCollapsed/endCollapsed are declared with reflect: true, but when collapsed state changes via toggle() / _handleExpanderAction() (i.e. by updating _collapsedPane), Lit will only consider _collapsedPane changed and will not re-reflect the start-collapsed / end-collapsed attributes. This can leave attributes stale (e.g. switching collapsed pane from start->end can keep start-collapsed present). Consider explicitly requesting updates for both public properties when _collapsedPane changes so attribute reflection stays consistent.
    this._collapsedPane = target;

    this._internals.setState('start-collapsed', this._isCollapsed('start'));
    this._internals.setState('end-collapsed', this._isCollapsed('end'));

src/components/splitter/splitter.ts:170

  • Same as the other @query() fields above: consider re-enabling cached queries to avoid repeated DOM lookups during interaction (consistent with other components, e.g. src/components/button/button-base.ts:54).
  @query('[part~="end-pane"]')
  private readonly _endPane!: HTMLElement;

Comment thread src/components/splitter/splitter.ts
Comment thread stories/splitter.stories.ts
Copilot AI review requested due to automatic review settings July 27, 2026 07:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

src/components/splitter/splitter.ts:163

  • @query('[part~="base"]') is no longer cached, which is inconsistent with other components that query a stable base part (e.g. src/components/tile-manager/tile.ts:189 uses @query('[part~="base"]', true)). Since <div part="base"> is always rendered here, caching avoids repeated DOM queries on each access and matches established pattern.
  @query('[part~="base"]')

stories/splitter.stories.ts:379

  • This demo reads persisted state with JSON.parse(saved) twice and will throw if the stored value is not valid JSON (or if localStorage is blocked). Parsing once and guarding errors makes the story more robust.
    const saved = localStorage.getItem(PERSISTED_LAYOUT_KEY);
    const startCollapsed = saved ? JSON.parse(saved).startCollapsed : false;
    const endCollapsed = saved ? JSON.parse(saved).endCollapsed : false;

Comment thread stories/splitter.stories.ts Outdated
Comment thread src/components/splitter/splitter.ts
Copilot AI review requested due to automatic review settings July 28, 2026 06:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

src/components/splitter/splitter.ts:498

  • The PR description says the feature "includes methods to collapse and expand panes", but the public API changes here only add startCollapsed/endCollapsed and reuse the existing toggle() method (no collapse() / expand() methods were added). Please either update the PR description or add the missing methods if they are part of the intended API.
  /** Toggles the collapsed state of the specified pane. */
  public toggle(position: PanePosition): void {
    // If the requested pane is already collapsed, expand it (set to null)
    // Otherwise, collapse the requested pane (this also handles switching from one collapsed pane to another)
    this._applyCollapse(this._collapsedPane === position ? null : position);
  }

stories/splitter.stories.ts:382

  • JSON.parse(saved) can throw if localStorage contains invalid/corrupted JSON for the key, which would break the Storybook story render. Consider a small try/catch fallback to treat invalid values as "no saved layout".
    const saved = localStorage.getItem(PERSISTED_LAYOUT_KEY);
    const layout = saved ? JSON.parse(saved) : null;

    const startSize = layout?.startSize ?? '50%';
    const startCollapsed = layout?.startCollapsed ?? false;

src/components/splitter/splitter.ts:508

  • startCollapsed/endCollapsed are computed from _collapsedPane, but _applyCollapse() mutates _collapsedPane directly (via toggle() and user interactions). Since Lit reflects attributes only for properties tracked as changed, start-collapsed / end-collapsed can become stale when collapse state changes without going through the corresponding property setters (e.g. clicking collapse buttons or calling toggle()). Trigger requestUpdate() for these reactive accessor properties when _collapsedPane changes so reflection stays in sync.
  private _applyCollapse(target: PanePosition | null): void {
    if (this._collapsedPane === null && target !== null) {
      this._savePaneSizes();
    }

…nged event

- Removed the igcExpansionChanged event in favor of a single igcLayoutChanged
  event, fired after a user-driven resize (drag/keyboard) or a collapse/expand
  toggle, carrying a full layout snapshot (startSize, endSize, startCollapsed,
  endCollapsed). Simplifies persisting/restoring layout to one listener instead
  of two.
- Fixed pane sizes collapsing to 'auto' when a pane is expanded after being
  collapsed programmatically before first render (e.g. restoring persisted
  state) - _savePaneSizes() now preserves the explicit size when layout can't
  be measured yet, instead of losing it.
- Fixed igcLayoutChanged reporting 'auto' for the still-expanded pane's size
  while the other pane is collapsed - both panes' sizes are forced to 'auto'
  together, so both must report from the saved pre-collapse size.
- Updated PersistedLayout story to use the single igcLayoutChanged listener
  and persist/restore startSize alongside the collapsed state.
- Cleaned up JSDoc formatting on splitter properties to match repo convention
  (tags immediately follow description, no blank line; corrected @deprecated
  format on IgcSplitterResizeEventDetail).
Copilot AI review requested due to automatic review settings August 4, 2026 09:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/components/splitter/splitter.ts:364

  • The new boolean property JSDoc uses “Gets/sets…”. In this codebase, boolean property docs generally describe the true state (e.g. “When true…/Whether…”). Updating this avoids inconsistent generated docs/Storybook metadata.
  /**
   * Gets/sets the collapsed state of the end pane.
   * @attr end-collapsed
   * @default false
   */

src/components/splitter/splitter.ts:503

  • The PR description mentions adding “methods to collapse and expand panes”, but the code changes introduce startCollapsed/endCollapsed (and reuse toggle()), without new collapse()/expand() public methods. Either update the PR description to match the implemented API, or add explicit methods if they’re part of the intended feature surface.
  /** Toggles the collapsed state of the specified pane. */
  public toggle(position: PanePosition): void {
    // If the requested pane is already collapsed, expand it (set to null)
    // Otherwise, collapse the requested pane (this also handles switching from one collapsed pane to another)
    this._applyCollapse(this._collapsedPane === position ? null : position);
  }

stories/splitter.stories.ts:462

  • JSON.parse(saved) can throw if localStorage contains invalid JSON, which would break the Storybook story rendering. Consider parsing defensively and falling back to defaults when the persisted value is corrupted.
    const saved = localStorage.getItem(PERSISTED_LAYOUT_KEY);
    const layout = saved ? JSON.parse(saved) : null;

src/components/splitter/splitter.ts:350

  • The new boolean property JSDoc uses “Gets/sets…”. In this codebase, boolean property docs generally describe the true state (e.g. “When true…/Whether…”). Updating this avoids inconsistent generated docs/Storybook metadata.

This issue also appears on line 360 of the same file.

  /**
   * Gets/sets the collapsed state of the start pane.
   * @attr start-collapsed
   * @default false
   */

Copilot AI review requested due to automatic review settings August 4, 2026 09:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/components/splitter/splitter.ts:520

  • startCollapsed/endCollapsed are declared as reflected properties, but their values are derived from _collapsedPane. When _collapsedPane changes (e.g. via toggle()/UI interactions), Lit will re-render because _collapsedPane is @state(), but it will not reflect start-collapsed/end-collapsed unless those properties are marked as changed via requestUpdate(). This can leave the reflected attributes stale after user-driven collapse/expand.
  private _applyCollapse(target: PanePosition | null): void {
    if (this._collapsedPane === null && target !== null) {
      this._savePaneSizes();
    }

    this._collapsedPane = target;

    this._internals.setState('start-collapsed', this._isCollapsed('start'));
    this._internals.setState('end-collapsed', this._isCollapsed('end'));

    this._restoreSizesOnExpandCollapse();
  }

stories/splitter.stories.ts:466

  • JSON.parse(saved) will throw if localStorage contains invalid/corrupted data for this key (which is easy to end up with during manual testing). That breaks the entire story render. Consider parsing defensively and falling back to defaults when parsing fails.
    const saved = localStorage.getItem(PERSISTED_LAYOUT_KEY);
    const layout = saved ? JSON.parse(saved) : null;

    const startSize = layout?.startSize ?? '50%';
    const startCollapsed = layout?.startCollapsed ?? false;
    const endCollapsed = layout?.endCollapsed ?? false;

Copilot AI review requested due to automatic review settings August 5, 2026 09:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/components/splitter/splitter.ts:367

  • Same wording issue as startCollapsed: prefer a boolean-state description (“When true…”) over “Gets/sets…”, to match existing docs and generated outputs.
  /**
   * Gets/sets the collapsed state of the end pane.
   * @attr end-collapsed
   * @default false
   */
  @property({ type: Boolean, reflect: true, attribute: 'end-collapsed' })
  public set endCollapsed(value: boolean) {
    this._setCollapsed('end', value);

stories/splitter.stories.ts:462

  • JSON.parse(saved) will throw if localStorage contains invalid JSON for this key, which can break the Storybook page entirely. It’s safer to parse with a try/catch and fall back to null.
    const saved = localStorage.getItem(PERSISTED_LAYOUT_KEY);
    const layout = saved ? JSON.parse(saved) : null;

src/components/splitter/splitter.ts:514

  • startCollapsed/endCollapsed are declared with reflect: true, but collapse changes triggered via toggle() or UI interactions only update _collapsedPane. Since Lit reflects attributes only for changed properties, the start-collapsed/end-collapsed attributes can get out of sync (e.g. switching from endCollapsed=true to startCollapsed=true may leave both attributes present). Consider explicitly requesting updates for both collapsed properties when _collapsedPane changes so reflection stays consistent.
    if (this._collapsedPane === null && target !== null) {
      this._savePaneSizes();
    }

    this._collapsedPane = target;

src/components/splitter/splitter.ts:354

  • The new boolean property docs use “Gets/sets …” wording, which is inconsistent with the rest of the component’s boolean API docs (e.g. “When true, …”). Since these descriptions flow into generated docs/Storybook, it’d be better to phrase them as a boolean state description.

This issue also appears on line 360 of the same file.

  /**
   * Gets/sets the collapsed state of the start pane.
   * @attr start-collapsed
   * @default false
   */
  @property({ type: Boolean, reflect: true, attribute: 'start-collapsed' })
  public set startCollapsed(value: boolean) {
    this._setCollapsed('start', value);

Copilot AI review requested due to automatic review settings August 5, 2026 10:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/components/splitter/splitter.ts:170

  • _base is queried frequently (e.g. via _getTotalSize() during resizing). Dropping the cached @query(..., true) causes repeated querySelector calls and is inconsistent with _startPane / _endPane (and other components) that cache stable part references.
  @query('[part~="base"]')
  private readonly _base!: HTMLElement;

stories/splitter.stories.ts:470

  • JSON.parse(saved) can throw if localStorage contains invalid/corrupted data, which would break the Storybook story rendering. It’s safer to guard parsing and fall back to defaults.
    const saved = localStorage.getItem(PERSISTED_LAYOUT_KEY);
    const layout = saved ? JSON.parse(saved) : null;

src/components/splitter/splitter.ts:506

  • The PR description says the feature "includes methods to collapse and expand panes", but the public API in this change exposes startCollapsed/endCollapsed and toggle(position) (no explicit collapse()/expand() methods). Either add the methods or update the PR description to match the actual shipped API.
  /** Toggles the collapsed state of the specified pane. */
  public toggle(position: PanePosition): void {
    // If the requested pane is already collapsed, expand it (set to null)
    // Otherwise, collapse the requested pane (this also handles switching from one collapsed pane to another)
    this._applyCollapse(this._collapsedPane === position ? null : position);

src/components/splitter/splitter.ts:517

  • startCollapsed/endCollapsed are reflected properties, but their values are derived from _collapsedPane. When _collapsedPane changes via user interaction (collapse buttons, keyboard) or toggle(), Lit will re-render due to the @state() change, but it will not automatically reflect the derived boolean attributes unless the properties are marked as changed. This can leave start-collapsed / end-collapsed attributes stale compared to the actual collapsed state.
  private _applyCollapse(target: PanePosition | null): void {
    if (this._collapsedPane === null && target !== null) {
      this._savePaneSizes();
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Add expansion state properties and an expansion changed event for Splitter panes

3 participants